Skip to content

HYPERFLEET-1182 - test: add nodepool integration tests for placement#208

Open
kuudori wants to merge 1 commit into
openshift-hyperfleet:mainfrom
kuudori:HYPERFLEET-1182
Open

HYPERFLEET-1182 - test: add nodepool integration tests for placement#208
kuudori wants to merge 1 commit into
openshift-hyperfleet:mainfrom
kuudori:HYPERFLEET-1182

Conversation

@kuudori

@kuudori kuudori commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Add integration tests backfilling coverage before E2E removal: TestNodePoolPatchSoftDeleted, TestNodePoolConcurrentDelete, TestNodePoolGetSoftDeleted, TestNodePoolListIncludesSoftDeleted.

Summary

  • HYPERFLEET-1182

Test Plan

  • Unit tests added/updated
  • make test-all passes
  • make lint passes
  • Helm chart changes validated with make test-helm (if applicable)
  • Deployed to a development cluster and verified (if Helm/config changes)
  • E2E tests passed (if cross-component or major changes)

@openshift-ci openshift-ci Bot requested review from ma-hill and rafabene June 8, 2026 18:46
@openshift-ci

openshift-ci Bot commented Jun 8, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sherine-k for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 023a8d41-3ea3-4801-aa32-efa846369121

📥 Commits

Reviewing files that changed from the base of the PR and between f515b2e and 77b5a6d.

📒 Files selected for processing (1)
  • test/integration/node_pools_test.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • openshift-hyperfleet/architecture (manual)
  • openshift-hyperfleet/hyperfleet-api (manual)
  • openshift-hyperfleet/hyperfleet-sentinel (manual)
  • openshift-hyperfleet/hyperfleet-adapter (manual)
  • openshift-hyperfleet/hyperfleet-broker (manual)
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/integration/node_pools_test.go
📝 Walkthrough

Summary by CodeRabbit

  • Tests
    • Added integration test coverage for soft-deleted node pool operations, including PATCH conflict handling, concurrent DELETE operations, GET responses, and LIST filtering to ensure API reliability and correct behavior across various scenarios.

Walkthrough

This PR adds four integration tests for nodepool soft-delete semantics in the Hyperfleet API. The changes verify: (1) PATCH operations on soft-deleted nodepools return 409 Conflict; (2) concurrent DELETE operations against the same target maintain idempotent generation and deleted_time values; (3) GET on a soft-deleted nodepool returns 200 OK with deleted_time in the response; (4) LIST by cluster includes both active and soft-deleted entries, with deleted_time set only for the soft-deleted one. A sync import is added to support concurrent test execution.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (10 passed)
Check name Status Explanation
Title check ✅ Passed The title references HYPERFLEET-1182 and mentions adding nodepool integration tests, which directly aligns with the changeset that adds four new integration test functions for nodepool soft-delete scenarios.
Description check ✅ Passed The description lists the four specific test functions being added (TestNodePoolPatchSoftDeleted, TestNodePoolConcurrentDelete, TestNodePoolGetSoftDeleted, TestNodePoolListIncludesSoftDeleted) and references the Jira ticket, matching the changeset content.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Sec-02: Secrets In Log Output ✅ Passed PR modifies only test/integration/node_pools_test.go (_test.go file), which is explicitly excluded from SEC-02 check scope. File contains no log/slog/logr/zap/fmt.Print calls in new tests.
No Hardcoded Secrets ✅ Passed No hardcoded secrets, API keys, tokens, passwords, private keys, or credentials found. All test data uses placeholder names; authentication properly abstracted via test helpers. Sync import is Go s...
No Weak Cryptography ✅ Passed No banned crypto primitives (md5, des, rc4, SHA1), custom crypto, ECB mode, or non-constant-time secret comparisons detected in test file.
No Injection Vectors ✅ Passed PR modifies only test file (test/integration/node_pools_test.go). Injection vector check explicitly excludes test files. No SQL injection (CWE-89), command injection (CWE-78), XSS (CWE-79), or dese...
No Privileged Containers ✅ Passed PR only adds test code to node_pools_test.go. No Kubernetes manifests, Helm templates, or Dockerfiles were modified. No privileged container settings found in existing deployment template.
No Pii Or Sensitive Data In Logs ✅ Passed Four new test functions added with zero logging statements. No slog, logr, zap, log, or fmt.Print* calls. All string literals contain only test data (nodepool names, assertion messages) with no PII...

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Comment @coderabbitai help to get the list of available commands and usage tips.

@hyperfleet-ci-bot

hyperfleet-ci-bot Bot commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown

Risk Score: 0 — risk/low

Signal Detail Points
PR size 163 lines +0
Sensitive paths none +0

Computed by hyperfleet-risk-scorer

@kuudori kuudori force-pushed the HYPERFLEET-1182 branch 2 times, most recently from e413e05 to f515b2e Compare June 9, 2026 13:07
@kuudori
HYPERFLEET-1182 - test: add nodepool integration tests for placement
77b5a6d 
Add integration tests backfilling coverage before E2E removal:
TestNodePoolPatchSoftDeleted, TestNodePoolConcurrentDelete,
TestNodePoolGetSoftDeleted, TestNodePoolListIncludesSoftDeleted.
@kuudori

kuudori commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ma-hill ma-hill Awaiting requested review from ma-hill

@rafabene rafabene Awaiting requested review from rafabene

Assignees

No one assigned

Labels

risk/low

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@kuudori